Repository navigation
Conversation
Tests PassedCommit: 1483657 |
| @@ -0,0 +1,9 @@ | |||
| { | |||
| pub fn minify_chunk_item( | ||
| code: &Code, | ||
| source_maps: bool, | ||
| mangle: Option<MangleType>, | ||
| ) -> Result<Code> { |
There was a problem hiding this comment.
We already have a
pub fn minify(code: Code, source_maps: bool, mangle: Option<MangleType>) -> Result<Code> {
function in the same file. How is this one different?
There was a problem hiding this comment.
The original minify was written to handle chunks with side-effectful code, so it avoids eliminating any expressions.
Since a module is just an arrow function, it has no side effect and the minifier tries to delete the statement as a no-op. To get around this, I modify the AST to assign the arrow function to a variable called __TURBOPACK_CHUNK_ITEM__, minify it, and then delete the assignment. This leaves the minified arrow function.
But you're right that I'm probably duplicating code to achieve this. PRobably the right approach is:
- move most of
minifyto a core that handles a pre-parsed AST minifybecomes a wrapper that parses the raw code and passes the AST to the implementationminify_chunk_itembecomes a wrapper that generates the artificial AST, then passes it to the inner implementation method
That, or I add an optional pre- and post-processing step to the AST in minify
fe1ba07 to
a1ac34b
Compare
a1ac34b to
060bad6
Compare
Minification happens on a per-chunk basis. With enough chunks, we get good parallelization, but a large chunk can create a single large span that wastes unused cores. This flag makes minification more granular, applying it to each module and caching the result. The goal is to make each minification step smaller, so that they can be evenly distributed across parallel workers. Theoretically this may also reduce the work done on a re-build, since the minification transform will only be applied to changed modules. The flag is configured behind an `experimental` config flag, and only applies to builds (because we don't minify in dev!). An ideal outcome is equal-or-faster builds, with almost no change to byte size. I don't expect much change to byte size, becuase the shape of our chunks doesn't lend itself to cross-module optimizations, but it's entirely possible we regress this a bit. That hypothesis will need to be tested against our benchmark suite of real Next apps. Co-authored-by: Andrew Imm <andrew.imm@vercel.com> Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>
060bad6 to
e0b7568
Compare
| } | ||
|
|
||
| #[test] | ||
| fn use_strict_directive_is_kept() { |
There was a problem hiding this comment.
some of these tests are just testing the swc behavior, we should drop them
| } | ||
|
|
||
| #[test] | ||
| fn use_strict_directive_can_be_stripped() { |
There was a problem hiding this comment.
so this is testing an SWC behavior where directives can be stripped if they don't change semantics?
| @@ -1,12 +1,10 @@ | |||
| (()=>{"use strict";(globalThis["TURBOPACK"] || (globalThis["TURBOPACK"] = [])).push(["output/1jsg_tests_snapshot_basic-tree-shake_import-side-effect_input_1_hdh9its1vdz._.js", | |||
| "[project]/turbopack/crates/turbopack-tests/tests/snapshot/basic-tree-shake/import-side-effect/input/index.js [test] (ecmascript)", ((__turbopack_context__) => { | |||
| "use strict";(globalThis.TURBOPACK||(globalThis.TURBOPACK=[])).push(["output/1jsg_tests_snapshot_basic-tree-shake_import-side-effect_input_1_hdh9its1vdz._.js","[project]/turbopack/crates/turbopack-tests/tests/snapshot/basic-tree-shake/import-side-effect/input/index.js [test] (ecmascript)",((__turbopack_context__) => { | |||
There was a problem hiding this comment.
im confused how use strict wound up here? does this need to be regenerated?
- Share one parse/optimize/print pipeline between `minify` and `minify_chunk_item`. - Wire `minify_before_chunking` into the Node.js chunking context and the Next.js server and edge chunking contexts (previously only client chunks used it). - Fix per-item minification dropping the additional module ids of merged (scope hoisted) factories (`id2, id3, <factory>` was parsed as a sequence and the ids compressed away). - Move the per-item vs whole-chunk decision into `ChunkMinification`. - Write chunk scaffolding in minified form in all modes, use a file-level `"use strict"` for all-strict chunks, and sort strict chunk items first. - In per-item mode, strip each strict factory's own directive and always group strict factories under a strict wrapper, and use deterministic mangling so names are consistent across factories. Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>
e0b7568 to
1483657
Compare
Minification happens on a per-chunk basis. With enough chunks, we get good parallelization, but a large chunk can create a single large span that wastes unused cores.
This flag makes minification more granular, applying it to each module and caching the result. The goal is to make each minification step smaller, so that they can be evenly distributed across parallel workers. Theoretically this may also reduce the work done on a re-build, since the minification transform will only be applied to changed modules.
The flag is configured behind an
experimentalconfig flag, and only applies to builds (because we don't minify in dev!).An ideal outcome is equal-or-faster builds, with almost no change to byte size. I don't expect much change to byte size, becuase the shape of our chunks doesn't lend itself to cross-module optimizations, but it's entirely possible we regress this a bit. That hypothesis will need to be tested against our benchmark suite of real Next apps.
Update: size measurements and follow-up changes
Changes in the second commit
id2, id3, (ctx) => {…}; it was parsed as a sequence expression and SWC compressed the ids away.next buildofbench/heavy-npm-depsfailed withmodule factory is not available. The ids are now split off before minifying and re-emitted unchanged.minifyandminify_chunk_itemshare a single parse/optimize/print pipeline (review feedback).NodeJsChunkingContext(the snapshot tests weren't exercising the per-item path before) and into the Next.js server and edge chunking contexts.(()=>{"use strict";…})()wrapper for all-strict chunks."use strict"handling: strict factories always keep their own directive, unminified and per-item minified alike, andCoderecords where it is (its own unmapped section). Chunk composition removes it only when the factory is inside a strict wrapper (CodeBuilder::push_code_without_strict_directive, using a new non-copyingRope::slice). If SWC drops a directive it proves unnecessary, nothing is recorded and nothing is removed. Execution tests (minification/strict-mode-layouts*) check strictness for flat, mixed and all-strict chunks in no-minify, whole-chunk and per-item modes.Deterministic(no char-frequency ordering), so names are consistent across factories and compress better.Output size: per-item vs whole-chunk minification
Current (canary
2420b9f578, with swc-project/swc#12580 and the"use strict"rework)Production
next buildof each app: A = canary, B = this branch flag off (whole-chunk), C = flag on (per-item). Sum of every.jsunder.next/static+.next/server, each file compressed separately (gzip -9, brotli -11). One run per variant.bench/nested-deps-app-router-many-pages(100 pages, 5.26 MB)bench/heavy-npm-deps(1.40 MB)"use strict"counts are now identical in A, B and C.606000where whole-chunk minification writes606e3.Previous (before the SWC fix, base
d8bfc3c994)Flag on vs off, minification enabled in both.
.jsunder.next/static+.next/server, gzip -9. next-swc was built from this branch in the debug profile (output bytes don't depend on the profile).bench/heavy-npm-deps, all JSbench/nested-deps-app-router-many-pages(100 pages), all JSWhere the original overhead came from (e.g. a 183 KB client chunk with 106 factories, +1,612 B):
"use strict"in every strict factory, since the chunk-level minifier normally strips these", ",globalThis["TURBOPACK"] || …)At the time, part of the gap was a whole-chunk bug: SWC dropped the directive of the mixed-mode strict IIFE (
[(()=>{"use strict";return[…]})(), …]), so strict factories ran sloppy and the baseline was smaller than correct output. swc-project/swc#12580 fixed this, and the numbers above are measured against the fixed baseline.